-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] fix(EditableText): optimize updateInputDimensions #4723
[core] fix(EditableText): optimize updateInputDimensions #4723
Conversation
@SatishGandham thanks for taking my suggestion and running with it. Looks like it is an incomplete implementation and will need to be adjusted / tweaked to get the right behavior. There are a few problems with this approach right now. (1) The input is not expanding its height past its minLines to accommodate longer text: (2) When switching to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see my feedback above
…s, so call updateInputDimensions when either change.
@adidahiya, Made the necessary changes. |
thanks @SatishGandham! |
Thank you @SatishGandham 🙌 Never got time to finish the implementation in #4639 myself. |
Checklist
Changes proposed in this pull request:
componentDidUpdate
of Editable text callsupdateInputDimensions
on every rerender, this triggers the layout calculations.This PR addresses that issue by only calling
updateInputDimensions
only when the props it depends on changeref: #4639 (comment)
Reviewers should focus on:
Confirm if calling
updateInputDimensions
conditionally is a safe change